✅ Deploy Preview for stacks-editor ready!

Name	Link
🔨 Latest commit	ea45a8c
🔍 Latest deploy log	https://app.netlify.com/sites/stacks-editor/deploys/67cb143da6bc110008bfe91b
😎 Deploy Preview	https://deploy-preview-389--stacks-editor.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

One regression I notice with this change is that we can end up with multiple spinners created by examplePreviewRenderer:

Maybe that was why there was this code for clearing out the preview in the first place?

I'm not sure if that means there is something to be fixed in examplePreviewRenderer itself, or if there needs to be some way of cancelling a call to the preview.renderer if there is one in-flight when a newer edit is made.

I'm not sure if that means there is something to be fixed in examplePreviewRenderer itself, or if there needs to be some way of cancelling a call to the preview.renderer if there is one in-flight when a newer edit is made.

Renderers are async functions: in the current implementation once we call them there is no way to cancel the side effects that are tasked to do from the editor codebase unfortunately. What we can do though is to pass in an AbortController signal as option that the consumers implementing the renderer can leverage to abort whatever they are doing.

This is how the update preview method will change (notice the addition of a new abortController class member)

private updatePreview(text: string) {
    this.container.innerHTML = "";

    // if showing the preview, fire off the renderer async
    if (this.isShown) {
        
       // PART OF CODE REMOVED BY THIS PR
           
        this.container.appendChild(this.dom);

        // Abort any previous render
        if (this.abortController) {
            this.abortController.abort();
        }

        // Create a new AbortController for the current render
        this.abortController = new AbortController();
        const signal = this.abortController.signal;

        void this.renderer?.(text, this.dom, { signal }).catch((e) => {
            if (e.name !== 'AbortError') {
                error(
                    "PreviewView.updatePreview",
                    `Uncaught exception in preview renderer`,
                    e
                );
            }
        });
    }
}

Then on the consumer end you can consume the signal. In the case of our examplePreviewRenderer in the site/index.ts file the part of sleepAsync(500) would become:

const interruptableSleepAsync = (delayMs, signal) => new Promise((resolve, reject) => {
        const timeoutId = setTimeout(() => {
            resolve();
        },  delayMs);

        signal.addEventListener("abort", () => {
            clearTimeout(timeoutId);
             reject(new Error('aborted'));
         });
 });
 
 interruptableSleepAsync(500, signal)
 .then(() => {
   // do what we were doing before
 })
 .catch(() => {
   // cleanup any lingering dom elements added previously if necessary (e.g. remove the spinner)
 })

Now you need to consider that in Core the renderer is different and a bit more sophisticated / legacy.
https://github.com/StackEng/StackOverflow/blob/main/StackOverflow/_Scripts/PartialJS/stacks-editor.mod.ts#L514-L520
Code will need to be adjusted in there so that the renderer can accept the signal and use it to prevent async side effects from happening when an in-flight render is aborted.

I am tagging @b-kelly since I don't know why we don't have already a mechanism to instruct consumers preview renderers to interrupt in-flight rendering. Perhaps we were relying on the fact that race conditions for async renders fired one after the other would be unlikely.

@giamir That looks like a nice pattern to interrupt the renderer, I will work on implementing that

Here's what the preview pane looks like with the AbortController at work @giamir @alexwarren

@alizaberger before investing time on implementing the mechanism to interrupt in-flight rendering it would be interesting to know if that would be useful in our Core context in my opinion.

You could go in Core to StackOverflow/node_modules/@stackoverflow/stacks-editor/dist/commonmark/plugins/preview.js and remove line 71 and 72 as you have done in this PR and then run npm start and check the specific page where the bug was reproduced.

Here's what the preview pane looks like with the AbortController at work @giamir @alexwarren

I think that's what I would expect based on how the examplePreviewRenderer is implemented in the demo site. We should not based our decision though on what that renderer but rather on what the renderer of Core does. This is why I was suggesting to check how your small changes behave in the Core context. If it does its job and doesn't have strange spinner appearing I would argue to keep the initial changes you made and adjust the implementation of the examplePreviewRenderer of the demo site so that we don't get that spinner cascade (perhaps we could remove the spinner all together from the renderer implementation). Hope my suggestion makes sense. If not feel free to DM me. :)

Final screen capture: